Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cargo-metadata): add workspace_default_members #11978

Merged

Conversation

charles-r-earp
Copy link
Contributor

What does this PR try to resolve?

Fixes #8033.

How should we test and review this PR?

I fixed one test "cargo_metadata_simple". Other tests haven't been updated, and potentially additional tests could be added to test the new functionality specifically.

Additional information

More tests would have to be updated. This change adds "workspace_default_members" to the ExportInfo struct which is serialized to json. I would imagine this could break existing testing infrastructure, but existing crates / apps reading this output should presumably expect that additional fields / data could be added.

@rustbot
Copy link
Collaborator

rustbot commented Apr 15, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added Command-metadata S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 15, 2023
@charles-r-earp charles-r-earp marked this pull request as draft April 15, 2023 23:59
@charles-r-earp charles-r-earp marked this pull request as ready for review April 22, 2023 20:03
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this!

Before moving forward, we will first solicit more info in #8033. Then probably do an FCP for seeking an agreement. Then review and merge this.

Once the feature is accepted, you will also need to update the documentation of cargo-metadata, like what 390af27 has done.

@weihanglo
Copy link
Member

Pardon my changing it to draft.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 26, 2023
@weihanglo weihanglo marked this pull request as draft April 26, 2023 12:00
@weihanglo
Copy link
Member

The team just agreed on adding this field 🎉.
@charles-r-earp, could you rebase onto master and update the doc as I mentioned earlier? Thanks in advance!

r? @weihanglo

@rustbot rustbot assigned weihanglo and unassigned ehuss May 3, 2023
@weihanglo weihanglo marked this pull request as ready for review May 3, 2023 00:29
@weihanglo weihanglo changed the title WIP Added workspace_default_members. feat(cargo-metadata): add workspace_default_members May 3, 2023
@charles-r-earp charles-r-earp force-pushed the metadata_workspace_default_members branch from c3345df to 8d579e4 Compare May 3, 2023 05:31
@rustbot rustbot added A-build-execution Area: anything dealing with executing the compiler A-cfg-expr Area: Platform cfg expressions A-cli-help Area: built-in command-line help A-console-output Area: Terminal output, colors, progress bar, etc. A-dependency-resolution Area: dependency resolution and the resolver A-documenting-cargo-itself Area: Cargo's documentation A-environment-variables Area: environment variables A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. A-interacts-with-crates.io Area: interaction with registries A-manifest Area: Cargo.toml issues A-profiles Area: profiles A-registries Area: registries A-registry-authentication Area: registry authentication and authorization (authn authz) labels May 3, 2023
@weihanglo weihanglo removed A-manifest Area: Cargo.toml issues A-testing-cargo-itself Area: cargo's tests A-cfg-expr Area: Platform cfg expressions A-semver Area: semver specifications, version matching, etc. A-cli-help Area: built-in command-line help Command-tree A-unstable Area: nightly unstable support A-registry-authentication Area: registry authentication and authorization (authn authz) A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. labels May 3, 2023
@weihanglo
Copy link
Member

Guess somewhat this pull request was messed up. You could either use git rebase and then force push, or pick up your changes and start over a new pull request.

Here are some references I found helpful:

Instructions to reviving this pull request are roughly like the followings:

git switch master
git pull origin master
git switch metadata_workspace_default_members
git switch --create backup
git switch -
git reset --hard master
git cherry-pick 8d579e4cd54b9e4437a6c7c13040090589e5bb73 48dcdfa5f11cfcd2b5e3f8de709fb3fc5afb93ac 5ff0fbd8588d2bd633010e24d9b4eb48fd9e16b4 4461032952bda70cab10cdfb8926e34a60bd955d
# Then create a new PR or force push to this PR, or continue working on it locally.

@charles-r-earp charles-r-earp force-pushed the metadata_workspace_default_members branch from 6d5bcb7 to f43e6d9 Compare May 3, 2023 19:47
@rustbot rustbot added A-cli-help Area: built-in command-line help A-documenting-cargo-itself Area: Cargo's documentation labels May 3, 2023
@charles-r-earp charles-r-earp requested a review from weihanglo May 3, 2023 21:15
@weihanglo
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented May 4, 2023

📌 Commit 61df6bb has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels May 4, 2023
@bors
Copy link
Contributor

bors commented May 4, 2023

⌛ Testing commit 61df6bb with merge 6240788...

@bors
Copy link
Contributor

bors commented May 4, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 6240788 to master...

@bors bors merged commit 6240788 into rust-lang:master May 4, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request May 5, 2023
Update cargo

10 commits in ac84010322a31f4a581dafe26258aa4ac8dea9cd..569b648b5831ae8a515e90c80843a5287c3304ef
2023-05-02 13:41:16 +0000 to 2023-05-05 15:49:44 +0000
- xtask-unpublished: output a markdown table (rust-lang/cargo#12085)
- fix: hack around `libsysroot` instead of `libtest` (rust-lang/cargo#12088)
- Optimize usage under rustup. (rust-lang/cargo#11917)
- Update lock to normalize `home` dep (rust-lang/cargo#12084)
- fix:  doc-test failures (rust-lang/cargo#12055)
- feat(cargo-metadata): add `workspace_default_members` (rust-lang/cargo#11978)
- doc: clarify implications of `cargo-yank` (rust-lang/cargo#11862)
- chore: Use `[workspace.dependencies]` (rust-lang/cargo#12057)
- support for shallow clones and fetches with `gitoxide` (rust-lang/cargo#11840)
- Build by PackageIdSpec, not name, to avoid ambiguity (rust-lang/cargo#12015)

r? `@ghost`
@ehuss ehuss added this to the 1.71.0 milestone May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli-help Area: built-in command-line help A-documenting-cargo-itself Area: Cargo's documentation A-workspaces Area: workspaces Command-metadata S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo metadata should return default workspace members
5 participants